Skip to content

Support expressions in confirm attribute#3201

Open
begoon wants to merge 5 commits intocasey:masterfrom
begoon:master
Open

Support expressions in confirm attribute#3201
begoon wants to merge 5 commits intocasey:masterfrom
begoon:master

Conversation

@begoon
Copy link
Copy Markdown
Contributor

@begoon begoon commented Mar 26, 2026

The confirmation prompt accepts any expression, so you can use variables,
concatenation, and function calls:

target := "production"
[confirm("Deploy to " + target + "?")]
deploy:
  echo 'Deploying...'

Recipe parameters can also be used in confirmation prompts:

[confirm("Deploy to " + env + "?")]
deploy env:
  echo 'Deploying to {{env}}...'

@begoon
Copy link
Copy Markdown
Contributor Author

begoon commented Mar 26, 2026

@casey

Copy link
Copy Markdown
Owner

@casey casey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Left some comments.

loop {
let name = self.parse_name()?;

let mut arguments = Vec::new();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing attributes is complex enough that I'd like to avoid this duplication. Can we always parse an expression, and then just complain if the expression is not a string literal unless it's the confirm attribute? That would avoid the duplication, and additionally make it easy to start accepting expressions in other attributes in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@casey Addressed


let mut evaluator = Evaluator::new(&context, BTreeMap::new(), true, &scope);

if !config.yes && !recipe.confirm(&mut evaluator)? {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmmmm, I kind of don't like that we now have to evaluate the parameters before we confirm that we want to run the recipe. This is a change in behavior. I think it's acceptable though, since there is probably not much going on in parameters expressions.

src/attribute.rs Outdated
}
}

impl Eq for Attribute<'_> {}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't these be derived?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@casey Addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants